Skip to content

Conversation

rcvalle
Copy link
Member

@rcvalle rcvalle commented Dec 8, 2022

This PR adds cross-language LLVM Control Flow Integrity (CFI) support to the Rust compiler by adding the -Zsanitizer-cfi-normalize-integers option to be used with Clang -fsanitize-cfi-icall-normalize-integers for normalizing integer types (see https://reviews.llvm.org/D139395).

It provides forward-edge control flow protection for C or C++ and Rust -compiled code "mixed binaries" (i.e., for when C or C++ and Rust -compiled code share the same virtual address space). For more information about LLVM CFI and cross-language LLVM CFI support for the Rust compiler, see design document in the tracking issue #89653.

Cross-language LLVM CFI can be enabled with -Zsanitizer=cfi and -Zsanitizer-cfi-normalize-integers, and requires proper (i.e., non-rustc) LTO (i.e., -Clinker-plugin-lto).

Thank you again, @bjorn3, @nikic, @samitolvanen, and the Rust community for all the help!

@rustbot
Copy link
Collaborator

rustbot commented Dec 8, 2022

r? @cjgillot

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 8, 2022
@@ -538,9 +514,31 @@ fn encode_ty<'tcx>(
// User-defined types
ty::Adt(adt_def, substs) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this misses #[repr(transparent)] support.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it in transform_ty? How exactly does that function relate to encode_ty and why is it split?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's handled in transform_ty, and its tests are here: https://github.com/rust-lang/rust/blob/master/src/test/codegen/sanitizer-cfi-emit-type-metadata-id-itanium-cxx-abi.rs#L116-L132. transform_ty coalesces types that have the same encoding into a single entity (e.g., c_void into unit), normalizes, and generalizes types before handing them to encode_ty. It greatly simplifies the encoding and compression/substitution logic.

@rcvalle rcvalle force-pushed the rust-cfi-3 branch 3 times, most recently from 8bc3f3d to de462bc Compare December 9, 2022 07:33
@cjgillot
Copy link
Contributor

r? @bjorn3

@rustbot rustbot assigned bjorn3 and unassigned cjgillot Dec 10, 2022
@rust-log-analyzer

This comment has been minimized.

@rcvalle rcvalle force-pushed the rust-cfi-3 branch 4 times, most recently from 4e6f11d to afdeab8 Compare December 14, 2022 07:17
@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented Dec 16, 2022

☔ The latest upstream changes (presumably #105763) made this pull request unmergeable. Please resolve the merge conflicts.

@rust-log-analyzer

This comment has been minimized.

@bjorn3 bjorn3 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 15, 2023
@Manishearth
Copy link
Member

@bors r=bjorn3

@bors
Copy link
Collaborator

bors commented May 3, 2023

📌 Commit 9a02f65 has been approved by bjorn3

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 3, 2023
Manishearth added a commit to Manishearth/rust that referenced this pull request May 3, 2023
Add cross-language LLVM CFI support to the Rust compiler

This PR adds cross-language LLVM Control Flow Integrity (CFI) support to the Rust compiler by adding the `-Zsanitizer-cfi-normalize-integers` option to be used with Clang `-fsanitize-cfi-icall-normalize-integers` for normalizing integer types (see https://reviews.llvm.org/D139395).

It provides forward-edge control flow protection for C or C++ and Rust -compiled code "mixed binaries" (i.e., for when C or C++ and Rust -compiled code share the same virtual address space). For more information about LLVM CFI and cross-language LLVM CFI support for the Rust compiler, see design document in the tracking issue rust-lang#89653.

Cross-language LLVM CFI can be enabled with -Zsanitizer=cfi and -Zsanitizer-cfi-normalize-integers, and requires proper (i.e., non-rustc) LTO (i.e., -Clinker-plugin-lto).

Thank you again, `@bjorn3,` `@nikic,` `@samitolvanen,` and the Rust community for all the help!
bors added a commit to rust-lang-ci/rust that referenced this pull request May 4, 2023
…earth

Rollup of 10 pull requests

Successful merges:

 - rust-lang#97594 (Implement tuple<->array convertions via `From`)
 - rust-lang#105452 (Add cross-language LLVM CFI support to the Rust compiler)
 - rust-lang#105695 (Replace generic thread parker with explicit no-op parker)
 - rust-lang#110371 (rustdoc: restructure type search engine to pick-and-use IDs)
 - rust-lang#110928 (tests: Add tests for LoongArch64)
 - rust-lang#110970 (tidy: remove ENTRY_LIMIT maximum checking and set it to 900)
 - rust-lang#111104 (Update ICU4X to 1.2)
 - rust-lang#111127 (Constify slice flatten method)
 - rust-lang#111146 (rustc_middle: Fix `opt_item_ident` for non-local def ids)
 - rust-lang#111154 (Use builtin FFX isolation for Fuchsia test runner)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 38bbc39 into rust-lang:master May 4, 2023
@rustbot rustbot added this to the 1.71.0 milestone May 4, 2023
@@ -329,6 +329,8 @@ declare_features! (
(active, cfg_target_thread_local, "1.7.0", Some(29594), None),
/// Allow conditional compilation depending on rust version
(active, cfg_version, "1.45.0", Some(64796), None),
/// Allows to use the `#[cfi_encoding = ""]` attribute.
(active, cfi_encoding, "1.69.0", Some(89653), None),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be CURRENT_RUSTC_VERSION?

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 15, 2023
…rrors

fixup version placeholder for `cfi_encoding` feature

Mentioned rust-lang#105452 (comment)
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request May 16, 2023
…rrors

fixup version placeholder for `cfi_encoding` feature

Mentioned rust-lang#105452 (comment)
Noratrieb added a commit to Noratrieb/rust that referenced this pull request May 16, 2023
…rrors

fixup version placeholder for `cfi_encoding` feature

Mentioned rust-lang#105452 (comment)
Noratrieb added a commit to Noratrieb/rust that referenced this pull request May 16, 2023
…rrors

fixup version placeholder for `cfi_encoding` feature

Mentioned rust-lang#105452 (comment)
RalfJung pushed a commit to RalfJung/miri that referenced this pull request May 18, 2023
fixup version placeholder for `cfi_encoding` feature

Mentioned rust-lang/rust#105452 (comment)
antoyo pushed a commit to antoyo/rust that referenced this pull request Jun 19, 2023
Add cross-language LLVM CFI support to the Rust compiler

This PR adds cross-language LLVM Control Flow Integrity (CFI) support to the Rust compiler by adding the `-Zsanitizer-cfi-normalize-integers` option to be used with Clang `-fsanitize-cfi-icall-normalize-integers` for normalizing integer types (see https://reviews.llvm.org/D139395).

It provides forward-edge control flow protection for C or C++ and Rust -compiled code "mixed binaries" (i.e., for when C or C++ and Rust -compiled code share the same virtual address space). For more information about LLVM CFI and cross-language LLVM CFI support for the Rust compiler, see design document in the tracking issue rust-lang#89653.

Cross-language LLVM CFI can be enabled with -Zsanitizer=cfi and -Zsanitizer-cfi-normalize-integers, and requires proper (i.e., non-rustc) LTO (i.e., -Clinker-plugin-lto).

Thank you again, ``@bjorn3,`` ``@nikic,`` ``@samitolvanen,`` and the Rust community for all the help!
rcvalle added a commit to rcvalle/rust that referenced this pull request Aug 25, 2023
Fix rust-lang#115150 by encoding f32 and f64 correctly for cross-language CFI. I
missed changing the encoding for f32 and f64 when I introduced the
integer normalization option in rust-lang#105452 as integer normalization does
not include floating point. `f32` and `f64` should be always encoded as
`f` and `d` since they are both FFI safe when their representation are
the same (i.e., IEEE 754) for both the Rust compiler and Clang.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 25, 2023
…piler-errors

Fix CFI: f32 and f64 are encoded incorrectly for cross-language CFI

Fix rust-lang#115150 by encoding f32 and f64 correctly for cross-language CFI. I missed changing the encoding for f32 and f64 when I introduced the integer normalization option in rust-lang#105452 as integer normalization does not include floating point. `f32` and `f64` should be always encoded as `f` and `d` since they are both FFI safe when their representation are the same (i.e., IEEE 754) for both the Rust compiler and Clang.
maurer pushed a commit to maurer/rust that referenced this pull request Apr 1, 2024
Fix rust-lang#115150 by encoding f32 and f64 correctly for cross-language CFI. I
missed changing the encoding for f32 and f64 when I introduced the
integer normalization option in rust-lang#105452 as integer normalization does
not include floating point. `f32` and `f64` should be always encoded as
`f` and `d` since they are both FFI safe when their representation are
the same (i.e., IEEE 754) for both the Rust compiler and Clang.
@rcvalle rcvalle deleted the rust-cfi-3 branch April 22, 2024 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.